Skip to content
This repository was archived by the owner on Apr 9, 2025. It is now read-only.

Translator on current builder #60

Merged
merged 9 commits into from
Apr 16, 2020
Merged

Translator on current builder #60

merged 9 commits into from
Apr 16, 2020

Conversation

humitos
Copy link
Member

@humitos humitos commented Apr 15, 2020

We set our own translator in the translators defined by other extensions, but
also for the current builder.

It may happen that there is an extension that defines two translators for
dirhml and json, and we are going to override them --the problem comes when
sphinx-build is called with -b custombuilder since we weren't overriding it
in that case.

This commit override the ones defined by the user, but also the default one.

NOTE: the diff is a little tricky. I only removed an indentation level (to remove the first if) and swap the for/if blocks. There is no other changes.

We set our own translator in the translators defined by other extensions, but
also for the current builder.

It may happen that there is an extension that defines two translators for
`dirhml` and `json`, and we are going to override them --the problem comes when
`sphinx-build` is called with `-b custombuilder` since we weren't overriding it
in that case.

This commit override the ones defined by the user, but also the default one.
@humitos humitos added the Needed: tests Tests are required label Apr 15, 2020
@humitos humitos requested a review from ericholscher April 15, 2020 19:31
Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This solved an issue I was having 👍

@humitos humitos removed the Needed: tests Tests are required label Apr 16, 2020
@humitos
Copy link
Member Author

humitos commented Apr 16, 2020

@ericholscher I updated this PR with,

  • added two tests to check that we are not modifying the translators
  • refactor where the test utils functions are defined to be able to share the logic
  • a small logic to never override the translators if it's not an html builder

humitos added 4 commits April 16, 2020 15:54
It does not make sense to override the domain classes when using other non-HTML
builders because there is nothing to add.

Although, due to a initialization race condition, we are not able to "not
override the Domains if the builder is non-HTML" so we need to skip our custom
resolution during build time.
@humitos humitos merged commit 935a660 into master Apr 16, 2020
@humitos humitos deleted the humitos/set-translator branch April 16, 2020 20:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants